Split register_tool into register_attribute_tool and register_lint_tool by nbdd0121 · Pull Request #158038 · rust-lang/rust

nbdd0121 · 2026-06-17T16:09:01Z

View all comments

Tracking issue: #66079

This implements the attribute parsing part of RFC 3808. The resolution part is yet to be implemented.

cc @jyn514

rustbot · 2026-06-17T16:09:05Z

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

rustbot · 2026-06-17T16:09:08Z

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 73 candidates
Random selection from 20 candidates

nbdd0121 · 2026-06-17T17:25:11Z

Error seems to be flaky rustdoc test: #157747

mejrs · 2026-06-17T17:30:23Z

r? me

(Or Jonathan I suppose)

mejrs · 2026-06-17T17:36:24Z

Can you expand on the "resolution yet to be implemented" thing? I was under the impression there would be no resolution and it'd all be, essentially, stringly typed.

nbdd0121 · 2026-06-17T17:55:17Z

Can you expand on the "resolution yet to be implemented" thing? I was under the impression there would be no resolution and it'd all be, essentially, stringly typed.

In https://github.com/rust-lang/rfcs/blob/master/text/3808-register-tool.md it mentioned that ambiguity between tool attribute and other resolution should be an error, and also that tool attribute is not affected by no_implicit_prelude. These are technically breaking changes so I'd want to split it to another PR so it gets a crater run.

mejrs

Typically when there are multiple attributes which are semantically related, we "merge" them into a single attribute. See https://doc.rust-lang.org/nightly/nightly-rustc/rustc_attr_parsing/index.html#how-this-crate-works

This implementation doesn't; you'd be dealing with RegisteredTool { kind: sym::register_lint_tool, ..}, RegisteredTool { kind: sym::register_tool, ..} and RegisteredTool { kind: sym::register_attribute_tool, ..}. That's three attributes, effectively. I've pointed at some places that need changes for that.

Also, my understanding from the rfc is that resolution and ambiguity errors are only for attribute_tool? i.e. #[my_tool::thing]? and not #[allow(my_tool::lint)]?

View changes since this review

mejrs · 2026-06-17T20:58:11Z

+pub(crate) struct RegisterAttributeTool;
+pub(crate) struct RegisterLintTool;
+pub(crate) struct RegisterTool;

-impl CombineAttributeParser for RegisterToolParser {
-    const PATH: &[Symbol] = &[sym::register_tool];
+pub(crate) trait RegisterToolKind: 'static {
+    const SYMBOL: Symbol;
+}
+
+impl RegisterToolKind for RegisterAttributeTool {
+    const SYMBOL: Symbol = sym::register_attribute_tool;
+}
+
+impl RegisterToolKind for RegisterLintTool {
+    const SYMBOL: Symbol = sym::register_lint_tool;
+}
+
+impl RegisterToolKind for RegisterTool {
+    const SYMBOL: Symbol = sym::register_tool;
+}
+
+pub(crate) struct RegisterToolParser<Kind>(Kind);
+
+impl<K: RegisterToolKind> CombineAttributeParser for RegisterToolParser<K> {
+    const PATH: &[Symbol] = &[K::SYMBOL];
    type Item = Ident;
-    const CONVERT: ConvertFn<Self::Item> = |tools, _span| AttributeKind::RegisterTool(tools);
+    const CONVERT: ConvertFn<Self::Item> =
+        |tools, _span| AttributeKind::RegisterTool { kind: K::SYMBOL, tools };


And this would need to implement AttributeParser directly. For example see the stability parser:

rust/compiler/rustc_attr_parsing/src/attributes/stability.rs

Line 64 in 693b3e4

impl AttributeParser for StabilityParser {

which combines #[stable], #[unstable] and #[rustc_allowed_through_unstable_modules] into a single semantic attribute.

mejrs · 2026-06-17T21:01:00Z


-    if let Some(Attribute::Parsed(AttributeKind::RegisterTool(tools))) =
+    if let Some(Attribute::Parsed(AttributeKind::RegisterTool { kind: _, tools })) =
+        AttributeParser::parse_limited(sess, pre_configured_attrs, &[sym])


Unfortunately you can only use parse_limited for one attribute at a time; you can't parse multiple attributes at a time. Feel free to make a function that allows multiple attributes though, if you need it.

petrochenkov · 2026-06-18T11:50:21Z

-    /// Tool modules introduced with `#![register_tool]`.
-    ToolPrelude,
+    /// Tool modules introduced with `#![register_tool]` or `#![register_attribute_tool]`.
+    ToolAttributePrelude,


Suggested change

ToolAttributePrelude,

AttributeToolPrelude,

It's a prelude for tools, not for attributes.

I'd probably just keep the ToolPrelude naming here, because lints do no go through name resolution, but may be better to be more explicit for readers less aware of the context.

View changes since the review

petrochenkov · 2026-06-18T12:02:51Z

+#![register_attribute_tool(qux)]
+#![register_attribute_tool(qux)]


@jyn514 is there a specific reason for

#![register_attribute_tool(qux)] #![register_attribute_tool(qux)]

to be allowed?
I don't see the explanation in the RFC.
Duplicate definitions are typically prohibited in name resolution.

View changes since the review

I am also curious on the reason. It is also strange to me that predefined tools cannot be duplicate defined. I feel that we should either allow duplicate definition for all tools (including predefined) or disallow it for all.

I don't recall why I did this. The one use case I can think of is to have register_attribute_tool at the crate root in the source, and also with --crate-attr passed to every crate in the workspace. If we errored on duplicates, we'd disallow that, which seems unfortunate. I'm mildly in favor of allowing duplicate definition for all tools, including pre-defined.

I have found it necessary on occasion to pass -Zcrate-attr=feature(foo) over command line when they are possibly declared in lib.rs as well. It's a deny by default lint to have duplicate features so it all just works if you pass allow(duplicate_features)

I'd expect register_tool to work similarly, but that need not be implemented in this PR

petrochenkov · 2026-06-18T12:05:28Z

Naming nit: "attribute" is long, so it's usually abbreviated to "attr" in the compiler, so all the AttributeToolPrelude/registered_attribute_tools internal stuff could be shortened to AttrToolPrelude/registered_attr_tools/etc.

petrochenkov · 2026-06-18T12:06:13Z

The resolution part is yet to be implemented.

Please, assign me when the resolution part is submitted.

jyn514 · 2026-07-22T08:03:32Z

Also, my understanding from the rfc is that resolution and ambiguity errors are only for attribute_tool? i.e. #[my_tool::thing]? and not #[allow(my_tool::lint)]?

correct, yes. allow(my_tool::lint) is unambiguous and does not require name resolution.

(caveats here about future work around proc-macro lints, but currently today they do not require name res.)

mejrs · 2026-07-26T21:48:12Z

Ping @nbdd0121 Can you resolve the merge conflicts and mark it ready for review if all comments (so far) have been addressed? It won't show up in my review queue if it isn't.

The existing API is preserved as `parse_limited_sym` as a simple helper. Similarly, `parse_limited_should_emit` is preserved as `parse_limited_sym_should_emit`. I've noted that all users targets crate, so the `target_node_id` and `target` parameters are removed from it.

mejrs

The impl looks good so far (I need to take a closer look at some parts - not done yet, will do tomorrow).

Can you add some tests for

the feature gate on the new attributes (can add to the existing one)
-Z crate-attr = <new attributes> over cmd line (can add to the existing one)

Also some things I noticed while reviewing. They need not be fixed in this PR - best in a standalone PR - but probably before you implement the nameres part:

register_tool is (and already was) a little too permissive in what is allowed. Things like #![register_tool(crate)] etc
there is no test to check that register_tool doesn't work across crates

View changes since this review

mejrs

r=me if perf is OK

@bors try @rust-timer queue

View changes since this review

Split register_tool into register_attribute_tool and register_lint_tool

mejrs · 2026-07-28T14:40:13Z

@rust-timer queue

mejrs · 2026-07-28T15:03:40Z

@bors try @rust-timer queue

Split register_tool into register_attribute_tool and register_lint_tool

rust-bors · 2026-07-28T17:13:37Z

☀️ Try build successful (CI)
Build commit: 17a90b8 (17a90b80266c5b23ffde14fa0033a6e41c452763)
Base parent: e19d321 (e19d321c06479c6fd77533582b0d5a86651f1be3)

rust-timer · 2026-07-28T17:53:53Z

Finished benchmarking commit (17a90b8): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 1.2%, secondary 0.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.5%	[3.5%, 3.5%]	1
Regressions ❌ (secondary)	4.9%	[4.9%, 4.9%]	1
Improvements ✅ (primary)	-1.0%	[-1.0%, -1.0%]	1
Improvements ✅ (secondary)	-1.2%	[-1.9%, -0.7%]	3
All ❌✅ (primary)	1.2%	[-1.0%, 3.5%]	2

Cycles

Results (secondary -1.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.2%	[2.7%, 4.1%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.2%	[-6.5%, -2.1%]	6
All ❌✅ (primary)	-	-	0

Binary size

Results (secondary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.0%]	4
All ❌✅ (primary)	-	-	0

Bootstrap: 490.165s -> 489.724s (-0.09%)
Artifact size: 388.34 MiB -> 390.42 MiB (0.54%)

mejrs · 2026-07-28T18:25:36Z

thanks for working on this :)

@bors r+ rollup

rust-bors · 2026-07-28T18:25:40Z

📌 Commit b0da120 has been approved by mejrs

It is now in the queue for this repository.

…uwer Rollup of 21 pull requests Successful merges: - #159990 (Many "predicate"-to-"clause" renamings) - #159665 (Replace most `Ty::new_fn_def` calls with `type_of` queries directly) - #159687 (rustdoc: Set tracing max_level_info when debug-logging is false) - #160057 (refactor(mir-transform): Calculate optimization status inside `run_passes_inner`) - #160060 (codegen: skip stores for entirely-uninit constant aggregate fields, attempt #2) - #160063 (Fix ICE when dumping the dep graph with the parallel frontend) - #160065 (Distinguish the dep-graph index space from the live node count) - #158038 (Split register_tool into register_attribute_tool and register_lint_tool) - #159776 (remove const hack in alloc) - #159978 (run intrinsic-test by default on x86_64-gnu) - #160008 (Avoid stale closure recovery state across statements) - #160027 (Add regression test for #132767) - #160030 (Update `browser-ui-test` version to `0.25.0`) - #160046 (Improve consistency of attribute error messages (part 2)) - #160056 (Fix associated function suggestion for generic ADTs) - #160069 (Update Rust crate tracing-subscriber to v0.3.23 [SECURITY]) - #160071 (sanitize_standard_fds: clarify macos comment) - #160076 (use unstable features when updating dependencies) - #160092 (miri ui tests: don't run native tests on stage 0) - #160093 (Switch cargo assignments to weihanglo) - #160094 (Update assignment for docs)

@jyn514

Rollup merge of #158038 - nbdd0121:register_tool, r=mejrs Split register_tool into register_attribute_tool and register_lint_tool Tracking issue: #66079 This implements the attribute parsing part of RFC 3808. The resolution part is yet to be implemented. cc @jyn514

…uwer Rollup of 21 pull requests Successful merges: - rust-lang/rust#159990 (Many "predicate"-to-"clause" renamings) - rust-lang/rust#159665 (Replace most `Ty::new_fn_def` calls with `type_of` queries directly) - rust-lang/rust#159687 (rustdoc: Set tracing max_level_info when debug-logging is false) - rust-lang/rust#160057 (refactor(mir-transform): Calculate optimization status inside `run_passes_inner`) - rust-lang/rust#160060 (codegen: skip stores for entirely-uninit constant aggregate fields, attempt rust-lang/rust#2) - rust-lang/rust#160063 (Fix ICE when dumping the dep graph with the parallel frontend) - rust-lang/rust#160065 (Distinguish the dep-graph index space from the live node count) - rust-lang/rust#158038 (Split register_tool into register_attribute_tool and register_lint_tool) - rust-lang/rust#159776 (remove const hack in alloc) - rust-lang/rust#159978 (run intrinsic-test by default on x86_64-gnu) - rust-lang/rust#160008 (Avoid stale closure recovery state across statements) - rust-lang/rust#160027 (Add regression test for rust-lang/rust#132767) - rust-lang/rust#160030 (Update `browser-ui-test` version to `0.25.0`) - rust-lang/rust#160046 (Improve consistency of attribute error messages (part 2)) - rust-lang/rust#160056 (Fix associated function suggestion for generic ADTs) - rust-lang/rust#160069 (Update Rust crate tracing-subscriber to v0.3.23 [SECURITY]) - rust-lang/rust#160071 (sanitize_standard_fds: clarify macos comment) - rust-lang/rust#160076 (use unstable features when updating dependencies) - rust-lang/rust#160092 (miri ui tests: don't run native tests on stage 0) - rust-lang/rust#160093 (Switch cargo assignments to weihanglo) - rust-lang/rust#160094 (Update assignment for docs)

rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 17, 2026

rustbot assigned wesleywiser Jun 17, 2026

This comment has been minimized.

Sign in to view

rustbot assigned mejrs and unassigned wesleywiser Jun 17, 2026

petrochenkov self-assigned this Jun 17, 2026

mejrs requested changes Jun 17, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 17, 2026

petrochenkov reviewed Jun 18, 2026

View reviewed changes

petrochenkov removed their assignment Jun 18, 2026

This comment has been minimized.

Sign in to view

petrochenkov mentioned this pull request Jun 22, 2026

Add new ambiguity error for tools #158146

Draft

nbdd0121 force-pushed the register_tool branch from 7d917c1 to bf55512 Compare July 8, 2026 14:13

This comment has been minimized.

Sign in to view

mejrs reviewed Jul 27, 2026

View reviewed changes

nbdd0121 added 2 commits July 27, 2026 20:05

Expand register_tool feature gate test to cover attr and lint tool

d0f008d

Add register_{attribute,lint}_tool test with crate-attr

b0da120

mejrs approved these changes Jul 28, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Jul 28, 2026

Auto merge of #158038 - nbdd0121:register_tool, r=<try>

7c37826

Split register_tool into register_attribute_tool and register_lint_tool

This comment was marked as outdated.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 28, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Jul 28, 2026

Auto merge of #158038 - nbdd0121:register_tool, r=<try>

17a90b8

Split register_tool into register_attribute_tool and register_lint_tool

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 28, 2026

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 28, 2026

JonathanBrouwer mentioned this pull request Jul 28, 2026

Rollup of 21 pull requests #160102

Merged

rust-bors Bot merged commit bca8564 into rust-lang:main Jul 28, 2026
14 checks passed

rustbot added this to the 1.99.0 milestone Jul 28, 2026

nbdd0121 deleted the register_tool branch July 29, 2026 11:43

nbdd0121 mentioned this pull request Jul 29, 2026

Expand checks for register_tool #160145

Open

		#![register_attribute_tool(qux)]
		#![register_attribute_tool(qux)]

Uh oh!

Uh oh!

Conversation

nbdd0121 commented Jun 17, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 17, 2026

Uh oh!

rustbot commented Jun 17, 2026

Uh oh!

This comment has been minimized.

nbdd0121 commented Jun 17, 2026

Uh oh!

mejrs commented Jun 17, 2026

Uh oh!

mejrs commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nbdd0121 commented Jun 17, 2026

Uh oh!

mejrs left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mejrs Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

mejrs Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petrochenkov Jun 18, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petrochenkov Jun 18, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nbdd0121 Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

jyn514 Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mejrs Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

petrochenkov commented Jun 18, 2026

Uh oh!

petrochenkov commented Jun 18, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jyn514 commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mejrs commented Jul 26, 2026

Uh oh!

mejrs left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mejrs left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment was marked as outdated.

mejrs commented Jul 28, 2026

Uh oh!

This comment was marked as outdated.

mejrs commented Jul 28, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

nbdd0121 commented Jun 17, 2026 •

edited by rustbot

Loading

mejrs commented Jun 17, 2026 •

edited

Loading

mejrs left a comment •

edited

Loading

mejrs Jun 17, 2026 •

edited

Loading

petrochenkov Jun 18, 2026 •

edited by rustbot

Loading

petrochenkov Jun 18, 2026 •

edited by rustbot

Loading

jyn514 Jul 22, 2026 •

edited

Loading

jyn514 commented Jul 22, 2026 •

edited

Loading

mejrs left a comment •

edited

Loading

mejrs left a comment •

edited by rustbot

Loading